Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Factor-out logic to determine the path of the precompilation cache file. #33173

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Sep 5, 2019

CUDAdrv.jl and LLVM.jl need to be able to wipe the precompilation cache file in order to force recompilation, since there's global state that depends on resp. the CUDA and LLVM library version. After #32651, that became a little cumbersome, so factor out the logic to get the path to the current cache file.

@maleadt maleadt added the packages Package management and loading label Sep 5, 2019
@maleadt
Copy link
Member Author

maleadt commented Sep 6, 2019

BSD failure is unrelated (tar permission denied during binary-dist).

@KristofferC
Copy link
Member

KristofferC commented Sep 6, 2019

CUDAdrv.jl and LLVM.jl need to be able to wipe the precompilation cache file in order to force recompilation

You have to be a bit careful here because after my PR we now have a "pool" of precompilation files and if any one of those matches, we will use that one. So is the plan for CUDAdrv.jl and LLVM.jl to remove all precompilation in that pool for the relevant packages?

Isn't it possible to use Base.include_dependency on some dummy file that the package touch to invalidate all precompilation file that declares a dependency on that file?

@maleadt
Copy link
Member Author

maleadt commented Sep 6, 2019

You have to be a bit careful here because after my PR we now have a "pool" of precompilation files and if any one of those matches, we will use that one. So is the plan for CUDAdrv.jl and LLVM.jl to remove all precompilation in that pool for the relevant packages?

Does it matter if I remove the current one or the entire pool? The removal happens during __init__ (after which an error is thrown), so if I only remove a single entry and the user switches projects afterwards, loading the package will just detect the inconsistency again and remove + recompile.

Isn't it possible to use Base.include_dependency on some dummy file that the package touch to invalidate all precompilation file that declares a dependency on that file?

That's an indirect alternative, yes, but IMO isn't much nicer than just directly removing the precompilation cache. Besides, avoiding global state (a tracked dummy file) is what lead me to doing all this work during precompilation in the first place, so I'd rather avoid re-introducing that. We kind-of used to do that by calling into Pkg.build and regenerating deps/build.jl, forcing recompilation, but that turned out to be problematic for clusters and containers (for other reasons as well).

@KristofferC
Copy link
Member

KristofferC commented Sep 6, 2019

Does it matter if I remove the current one or the entire pool? The removal happens during init (after which an error is thrown), so if I only remove a single entry and the user switches projects afterward, loading the package will just detect the inconsistency again and remove + recompile.

I am not sure exactly how the deletion is done so everything might be fine. I was just thinking of the case where you had a precompile from another environment that is valid even for the current project. So if you delete the precompilation file that the current environment would use, there might still be one in the pool that matches and you don't get a recompilation. But maybe that worry is not applicable here.

@maleadt maleadt merged commit 7cafa94 into master Sep 6, 2019
@delete-merged-branch delete-merged-branch bot deleted the tb/cachefile branch September 6, 2019 09:40
@maleadt
Copy link
Member Author

maleadt commented Sep 6, 2019

I am not sure exactly how the deletion is done so everything might be fine.

Pretty sure it's fine how we do it, since __init__ will be triggered after loading a different precompilation file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants